Make collapsible_if recognize the let_chains feature
#14481
Conversation
rustbot
added
the
S-waiting-on-review
samueltardieu
force-pushed
the
push-mkzzlozzzxup
branch
from
dee48da to
cae2375
Compare
| /// If `block` is a block with either one expression or a statement containing an expression, | ||
| /// return the expression. | ||
| fn expr_block<'tcx>(block: &Block<'tcx>) -> Option<&'tcx Expr<'tcx>> { | ||
| match block.stmts { | ||
| [] => block.expr, | ||
| [stmt] => { | ||
| if let StmtKind::Semi(expr) = stmt.kind { | ||
| Some(expr) | ||
| } else { | ||
| None | ||
| } | ||
| }, | ||
| _ => None, | ||
| } | ||
| } |
There was a problem hiding this comment.
Could this better be implemented with clippy_utils::peel_blocks and then just "unpack" the statement or return the expression?
Or are you intentionally only removing 1 block?
There was a problem hiding this comment.
I am intentionally removing only one block, the top-level one in the then parts, as multiple blocks would probably indicate a design choice, and ought to be flagged by another lint if this is not needed.
There was a problem hiding this comment.
Makes sense. In that case, please extend the comment to note that this is intentionally not using peel_blocks
clippy_lints/src/collapsible_if.rs
Outdated
| pub fn new(conf: &'static Conf) -> Self { | ||
| pub fn new(tcx: TyCtxt<'_>, conf: &'static Conf) -> Self { | ||
| Self { | ||
| let_chains_enabled: tcx.features().enabled(sym::let_chains), |
There was a problem hiding this comment.
| let_chains_enabled: tcx.features().enabled(sym::let_chains), | |
| let_chains_enabled: tcx.features().let_chains(), |
I think this is the better approach. With that, the person stabilizing this feature in rustc is also forced to remove this check.
Yes, totally automated, with the addition of this additional whitespace reformatting on top at a few places where |
samueltardieu
force-pushed
the
push-mkzzlozzzxup
branch
3 times, most recently
from
1bf1a03 to
dee46f2
Compare
Until `if let` chains are stabilized, we do not collapse them together or with other `if` expressions unless the `let_chains` feature is enabled. This is the case for example in Clippy sources.
Since Clippy uses the `let_chains` feature, there are many occasions to collapse `if` and `if let` statements.
samueltardieu
force-pushed
the
push-mkzzlozzzxup
branch
from
dee46f2 to
79c6911
Compare
* Code Quality Improvements: - Eliminate LLM produced garbage code comments (In my defense, I were taking all nighters) - Eliminate duplication - Add constants: `MIN_BUFFER_MB`, `RAM_SAFETY_FACTOR` (was 0.8, now 0.9) - Apply `ln_1p()` for better float precision - Fix `collapsible_if` warnings using let-chains (rust-lang/rust-clippy#14481) - Replace deprecated `criterion::black_box` with `std::hint::black_box` - Avoid unnecessary clones: pass `&str` to worker threads
3 participants
Until
if letchains are stabilized, we do not collapse them together or with otherifexpressions unless thelet_chainsfeature is enabled. This is the case for example in Clippy sources.This was made possible by converting the
collapsible_ifto a late lint to get access to the set of enabled features. This allows this PR to supersede #14455 and no longer require an additional configuration option.The three commits are, in order:
let_chainsfeature detection and action, and testschangelog: [
collapsible_if]: recognize the rust compilerlet_chainsfeaturer? @flip1995